-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[LLVM] Simplify GPU runtimes flag handling #159802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary: The AMDGPU hack can be removed, and we no longer need to skip 90% of the `HandleLLVMOptions` if we work around NVPTX earlier. Simplifies the interface by removing duplicated logic and keeps the GPU targets from being weirdly divergent on some flags.
@llvm/pr-subscribers-libc Author: Joseph Huber (jhuber6) ChangesSummary: Full diff: https://github.com/llvm/llvm-project/pull/159802.diff 6 Files Affected:
diff --git a/compiler-rt/cmake/Modules/CompilerRTUtils.cmake b/compiler-rt/cmake/Modules/CompilerRTUtils.cmake
index 03db38fa4cdc1..cbd18d26c0b93 100644
--- a/compiler-rt/cmake/Modules/CompilerRTUtils.cmake
+++ b/compiler-rt/cmake/Modules/CompilerRTUtils.cmake
@@ -418,11 +418,9 @@ macro(construct_compiler_rt_default_triple)
# Pass the necessary flags to make flag detection work.
if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "amdgcn")
set(COMPILER_RT_GPU_BUILD ON)
- set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nogpulib")
elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "nvptx")
set(COMPILER_RT_GPU_BUILD ON)
- set(CMAKE_REQUIRED_FLAGS
- "${CMAKE_REQUIRED_FLAGS} -flto -c -Wno-unused-command-line-argument")
+ set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -flto -c")
endif()
endif()
diff --git a/flang-rt/CMakeLists.txt b/flang-rt/CMakeLists.txt
index a45a66be6f833..cad39d0c71016 100644
--- a/flang-rt/CMakeLists.txt
+++ b/flang-rt/CMakeLists.txt
@@ -221,15 +221,6 @@ endif()
# System Introspection #
########################
-# The GPU targets require a few mandatory arguments to make the standard CMake
-# check flags happy.
-if ("${LLVM_RUNTIMES_TARGET}" MATCHES "^amdgcn")
- set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nogpulib")
-elseif ("${LLVM_RUNTIMES_TARGET}" MATCHES "^nvptx")
- set(CMAKE_REQUIRED_FLAGS
- "${CMAKE_REQUIRED_FLAGS} -flto -c -Wno-unused-command-line-argument")
-endif()
-
include(CheckCXXSymbolExists)
include(CheckCXXSourceCompiles)
check_cxx_symbol_exists(strerror_r string.h HAVE_STRERROR_R)
diff --git a/libc/cmake/modules/prepare_libc_gpu_build.cmake b/libc/cmake/modules/prepare_libc_gpu_build.cmake
index df1de1454c3eb..4d12a5917a56f 100644
--- a/libc/cmake/modules/prepare_libc_gpu_build.cmake
+++ b/libc/cmake/modules/prepare_libc_gpu_build.cmake
@@ -20,12 +20,6 @@ endif()
if(LIBC_TARGET_TRIPLE)
set(CMAKE_REQUIRED_FLAGS "--target=${LIBC_TARGET_TRIPLE}")
endif()
-if(LIBC_TARGET_ARCHITECTURE_IS_AMDGPU)
- set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nogpulib")
-elseif(LIBC_TARGET_ARCHITECTURE_IS_NVPTX)
- set(CMAKE_REQUIRED_FLAGS
- "${CMAKE_REQUIRED_FLAGS} -flto -c -Wno-unused-command-line-argument")
-endif()
# Optionally set up a job pool to limit the number of GPU tests run in parallel.
# This is sometimes necessary as running too many tests in parallel can cause
diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 24142c934b918..ba82f72df4f1e 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -144,12 +144,6 @@ if( LLVM_ENABLE_ASSERTIONS )
endif()
endif()
-# If we are targeting a GPU architecture in a runtimes build we want to ignore
-# all the standard flag handling.
-if(LLVM_RUNTIMES_GPU_BUILD)
- return()
-endif()
-
if(LLVM_ENABLE_EXPENSIVE_CHECKS)
# When LLVM_ENABLE_EXPENSIVE_CHECKS is ON, LLVM will intercept errors
# using assert(). An explicit check is performed here.
diff --git a/openmp/CMakeLists.txt b/openmp/CMakeLists.txt
index 1358e896d3cc5..a2b7e1054e02b 100644
--- a/openmp/CMakeLists.txt
+++ b/openmp/CMakeLists.txt
@@ -99,14 +99,6 @@ else()
set(CMAKE_CXX_EXTENSIONS NO)
endif()
-# Targeting the GPU directly requires a few flags to make CMake happy.
-if("${CMAKE_CXX_COMPILER_TARGET}" MATCHES "^amdgcn")
- set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nogpulib")
-elseif("${CMAKE_CXX_COMPILER_TARGET}" MATCHES "^nvptx")
- set(CMAKE_REQUIRED_FLAGS
- "${CMAKE_REQUIRED_FLAGS} -flto -c -Wno-unused-command-line-argument")
-endif()
-
# Check and set up common compiler flags.
include(config-ix)
include(HandleOpenMPOptions)
diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index 45a6517737b95..fac621a076aa4 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -181,15 +181,14 @@ if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
endif()
endif()
+# The NVPTX target needs to override linking to pass compiler flag checks.
+if("${LLVM_RUNTIMES_TARGET}" MATCHES "^nvptx")
+ set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -flto -c")
+endif()
+
# Avoid checking whether the compiler is working.
set(LLVM_COMPILER_CHECKED ON)
-# This can be used to detect whether we're targeting a GPU architecture.
-if("${LLVM_RUNTIMES_TARGET}" MATCHES "^amdgcn" OR
- "${LLVM_RUNTIMES_TARGET}" MATCHES "^nvptx64")
- set(LLVM_RUNTIMES_GPU_BUILD ON)
-endif()
-
# Handle common options used by all runtimes.
include(AddLLVM)
include(HandleLLVMOptions)
|
libcxx failure are unrelated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from the libc side
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/18/builds/21326 Here is the relevant piece of the build log for the reference
|
Summary: The AMDGPU hack can be removed, and we no longer need to skip 90% of the `HandleLLVMOptions` if we work around NVPTX earlier. Simplifies the interface by removing duplicated logic and keeps the GPU targets from being weirdly divergent on some flags.
Summary:
The AMDGPU hack can be removed, and we no longer need to skip 90% of the
HandleLLVMOptions
if we work around NVPTX earlier. Simplifies theinterface by removing duplicated logic and keeps the GPU targets from
being weirdly divergent on some flags.